test(table): split the cell-render suite by concern - #376
Merged
Conversation
… prose Review found four claims in the new headers that the tree contradicts: - the fixtures header said render-map never touches innerHTML, but cm-table-cell-render-map.test.ts:275 asserts on it. The conclusion (that suite does not need the shared serialiser) was right, the reason was not. - the urls header said both at-cap rows observe a click. Only the absolute arm does; the autolink arm asserts liveness and stops. The header now records why that modifier-click loop must stay: the clicks suite only ever uses short URLs, so it is the sole pin on at-cap x modifier-click routing. - the inline-ir header justified its happy-dom pragma as how every file in the directory declares its environment. cm-table-fallback-warn.test.ts carries none, and vitest.config.ts sets node globally, so the pragma is a per-file opt-in rather than a declaration. - a cross-file reference dropped the .test segment, so following it landed on nothing. Pre-existing in the pre-split file, corrected here because every other such reference in the six new suites spells the suffix out.
The per-file 'Fixtures: helpers/cell-render-fixtures.ts' line repeated what the import directly below it already states, and the fixtures header carried a rhetorical aside that added no information the preceding clause lacked.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Splits
test/webview/table/cm-table-cell-render.test.ts(1314 lines, 169 tests) into six concern-named suites, and extracts the two helpers they share. Follow-up to #372, which split the widget suite in the same directory.describenamescm-table-cell-render-urls.test.tsMAX_HREF_LENGTHcap, relative-image containmentcm-table-cell-render-emphasis.test.tscm-table-cell-render-text.test.tscm-table-cell-render-clicks.test.tsauxclick/contextmenurouting on a live linkcm-table-cell-inline-ir.test.tsparseCellInlinespan losslessnesscm-table-cell-render-map.test.tsrenderCellIntoemitsWhy these groupings
Unlike #372, this file did not cut along existing
describeboundaries — one 777-linedescribe("renderCellInline")held ~90 flatits spanning five unrelated contracts, so the grouping had to be decided rather than derived.Two calls worth flagging:
*,_,~~and==are emitted as delimiter runs into the same stack (inline-emphasis.ts) and paired by oneresolveInlinepass. Several rows assert exactly that coupling (*a~~b*c~~d*,*a==b*c==d*,~~a ==b== c~~); splitting marks into their own file would leave those rows with no home that owns both sides.MAX_HREF_LENGTHcap lives with the URL gate, not the click routing, because it decides whether an anchor is created at all. Two of its rows also observe a click, but only as proof the anchor really went live.One row was relocated rather than carried:
renders a pathologically deep-emphasis cell without crashingsat inside the text-node topologydescribebut asserts the walker stopping atMAX_INLINE_NESTING_DEPTH— the delimiter stack's termination behaviour, not text merging. It moves to the emphasis suite.Shared helpers
html()(85 call sites) and the tooltip-stripping regex (11 occurrences) move tohelpers/cell-render-fixtures.ts, matching thehelpers/widget-fixtures.tsprecedent. Two hardenings came with sharing:html()appends clones. It previouslyappendChilded the caller's nodes, moving them out of the array the caller still held. No call site reads its nodes after serialising them, so this changes no result — it removes a trap a shared serialiser would set for the first test that wants to check markup and then dispatch an event.titleattribute, so a meaningful title added later cannot vanish from every structural snapshot silently. The tooltip's own contract stays pinned where it already is: the twoa.titlerows in the clicks suite.How conservation was proven
The split was generated from a pinned blob, not hand-edited, so "no test was lost, duplicated or quietly reworded" is checkable rather than asserted:
The checks were mutation-tested: dropping a body line, corrupting one, extending a range, and breaking a reflow's source block each fail a different guard.
Verification
pnpm test:unit— 5041 passed, unchanged from before the split (files 259 → 264)pnpm compileclean;pnpm lintexit 0 (remaining warnings are pre-existing, in a file this PR does not touch)test/webview/table/exceeds ~600 lines; the largest is unchanged at 504Notes
Test-only. Nothing under
src/is modified.One comment repoint was necessary: a comment reading "the dedicated tooltip test below" moved to the urls suite while the tooltip test it referenced moved to clicks. It now names the clicks suite. All ten positional comment references were checked; that was the only one crossing a file boundary. A stale
cm-table-cell-renderreference instyles-contract.test.tsis repointed to the module it actually meant,cell-render.ts.